[SYCL] Disable dead arg elimination for free function kernels #19776
Conversation
This reverts commit 34a11ca.
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
I've removed reference to the internal bug report from the PR's description. That's not how upstream development should be done. That said, the rest of the description was great and provides enough context for the change, so no additional actions needed. |
|
Something is wrong with codeownership. I'd expect |
… of https://github.com/lbushi25/llvm into disable_dead_arg_elimination_for_free_function_kernels
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
Re-requested review since this is high priority in the context of the next release. |
| // Variadic template functions do not work with free function kernels. See | ||
| // CMPLRLLVM-69528. | ||
| // TODO: Uncomment the following line once the tracker is resolved. | ||
| // test_func<variadic_templated<double>, double>(); |
There was a problem hiding this comment.
I'll leave review of this to @AlexeySachkov.
There was a problem hiding this comment.
Is it a regression from this patch, or is it something previously known?
There was a problem hiding this comment.
Is it a regression from this patch, or is it something previously known?
I believe it's previously known. The failure described by the tracker has been happening since the test was created, it just wasn't caught by pre-commit since it only manifests with optimizations disabled.
There was a problem hiding this comment.
Is it a regression from this patch, or is it something previously known?
I believe it's previously known. The failure described by the tracker has been happening since the test was created, it just wasn't caught by pre-commit since it only manifests with optimizations disabled.
Ideally, such test should be disabled in a separate PR not to cause confusion, but I'm fine with the change since we do have a tracker to analyze the issue
| // Variadic template functions do not work with free function kernels. See | ||
| // CMPLRLLVM-69528. | ||
| // TODO: Uncomment the following line once the tracker is resolved. | ||
| // test_func<variadic_templated<double>, double>(); |
There was a problem hiding this comment.
Is it a regression from this patch, or is it something previously known?
Co-authored-by: Alexey Sachkov <[email protected]>
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
Both failing pre-commit jobs are failing at downloading the toolkit. Weirdly, re-running does not seem to fix it so it seems to be some persistent infrastructure bug. Either way, not related to my changes for sure. @intel/llvm-gatekeepers Can you merge this please? |
Seems like one more review needed for this to get merged. Ignore for now please! |
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
Failures unrelated, see #19767 |
This PR disables dead argument elimination for free function kernels. When using a free function kernel, the user sets the arguments manually using
handler::set_argbut if certain arguments are optimized away in case they are not used, the implementation needs to be aware of this and remove the relevantset_argcalls. Instead, its much more feasible to just disable dead arg elimination in this case.